Fix GroundHeatExchangerXXX cloning to another model#5460
Fix GroundHeatExchangerXXX cloning to another model#5460joseph-robertson wants to merge 9 commits intodevelopfrom
Conversation
joseph-robertson
added
severity - Minor Bug
component - Model
Pull Request - Ready for CI
| auto ghClone = gh.clone(m).cast<GroundHeatExchangerHorizontalTrench>(); | ||
| EXPECT_EQ(2u, m.getConcreteModelObjects<GroundHeatExchangerHorizontalTrench>().size()); | ||
| EXPECT_EQ(1u, m.getConcreteModelObjects<SiteGroundTemperatureUndisturbedKusudaAchenbach>().size()); | ||
| EXPECT_EQ(2u, m.getConcreteModelObjects<SiteGroundTemperatureUndisturbedKusudaAchenbach>().size()); |
There was a problem hiding this comment.
Should clone into the same model create a new SGT object here? If not, how can this be prevented?
There was a problem hiding this comment.
I don't see a precedent for determining whether to clone based on the model that the GHE is in. And I don't see an issue with just cloning into the same model.
| auto ghClone = gh.clone(m).cast<GroundHeatExchangerHorizontalTrench>(); | ||
| EXPECT_EQ(2u, m.getConcreteModelObjects<GroundHeatExchangerHorizontalTrench>().size()); | ||
| EXPECT_EQ(1u, m.getConcreteModelObjects<SiteGroundTemperatureUndisturbedKusudaAchenbach>().size()); | ||
| EXPECT_EQ(2u, m.getConcreteModelObjects<SiteGroundTemperatureUndisturbedKusudaAchenbach>().size()); |
There was a problem hiding this comment.
I don't see a precedent for determining whether to clone based on the model that the GHE is in. And I don't see an issue with just cloning into the same model.
| std::vector<ModelObject> GroundHeatExchangerHorizontalTrench_Impl::children() const { | ||
| std::vector<ModelObject> result; | ||
|
|
||
| result.push_back(undisturbedGroundTemperatureModel()); | ||
|
|
||
| return result; | ||
| } |
There was a problem hiding this comment.
It this new children method necessary? I'm still not really sure how this method gets used downstream.
|
CI Results for 786f151:
|
2 participants
Pull request overview
Pull Request Author
src/model/test)src/energyplus/Test)src/osversion/VersionTranslator.cpp)Labels:
IDDChangeAPIChangePull Request - Ready for CIso that CI builds your PRReview Checklist
This will not be exhaustively relevant to every PR.